New Module: docker_network #3728

keitwb · 2016-05-24T12:47:59Z

ISSUE TYPE

New Module Pull Request

COMPONENT NAME

docker_network

ANSIBLE VERSION

ansible 2.2.0
  config file = 
  configured module search path = Default w/o overrides

SUMMARY

Implementation of https://github.com/ansible/proposals/blob/master/docker/docker_network.md

{
    "changed": true, 
    "invocation": {
        "module_args": {
            "api_version": null, 
            "cacert_path": null, 
            "cert_path": null, 
            "connected": [
                "ansible-dev", 
                "cm_db_1"
            ], 
            "debug": false, 
            "docker_host": null, 
            "driver": "bridge", 
            "driver_options": {
                "com.docker.network.bridge.name": "test3"
            }, 
            "filter_logger": false, 
            "force": false, 
            "incremental": false, 
            "ipam_driver": null, 
            "ipam_options": {
                "gateway": "172.3.26.1", 
                "iprange": "192.168.1.0/24", 
                "subnet": "172.3.26.0/16"
            }, 
            "key_path": null, 
            "network_name": "ansible-test-3", 
            "ssl_version": null, 
            "state": "present", 
            "timeout": null, 
            "tls": null, 
            "tls_hostname": null, 
            "tls_verify": null
        }
    }, 
    "facts": {
        "Containers": {
            "cee84d31d7f7813c639ff231c297802f45b04e39bb10de36253ffdfae40ad85b": {
                "EndpointID": "8ad46af0d70b92c10cff36dd30b8a6269124980a3318ea5753c8a51cff635c9a", 
                "IPv4Address": "172.3.1.1/16", 
                "IPv6Address": "", 
                "MacAddress": "02:42:ac:03:01:01", 
                "Name": "cm_db_1"
            }, 
            "fe86a4569bd6af468d405050429a1f59712fab019569dd3d1dc42407e432047c": {
                "EndpointID": "5d0eedb1db198ca8ac4c8de4beebc0ffc9c230faa1cb2ec842d7a520912715ef", 
                "IPv4Address": "172.3.1.0/16", 
                "IPv6Address": "", 
                "MacAddress": "02:42:ac:03:01:00", 
                "Name": "ansible-dev"
            }
        }, 
        "Driver": "bridge", 
        "EnableIPv6": false, 
        "IPAM": {
            "Config": [
                {
                    "Gateway": "172.3.26.1", 
                    "IPRange": "192.168.1.0/24", 
                    "Subnet": "172.3.26.0/16"
                }
            ], 
            "Driver": "default", 
            "Options": null
        }, 
        "Id": "277c72ec582c8ac0366734b34605ae5ae6a22b918d720c610d3efd3564c75614", 
        "Internal": false, 
        "Labels": {}, 
        "Name": "ansible-test-3", 
        "Options": {
            "com.docker.network.bridge.name": "test3"
        }, 
        "Scope": "local"
    }
}

gregdek · 2016-05-24T15:25:11Z

Thanks @keitwb for this PR. This PR requires revisions, either because it fails to build or by reviewer request. Please make the suggested revisions. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review.

[This message brought to you by your friendly Ansibull-bot.]

chouseknecht · 2016-05-25T10:50:53Z

cloud/docker/docker_network.py

+
+def main():
+    argument_spec = dict(
+        network_name       = dict(type='str', required=True),


Needs aliases=['name']

chouseknecht · 2016-05-25T11:12:09Z

Overall this is looking good. If you can correct the minor things I pointed out, I think we can get this merged.

Nice job!

keitwb · 2016-05-25T12:30:43Z

Thanks, I cranked the required docker-py version requirement up to 1.7+ since that is when they added the ipam support when creating a network.

keitwb · 2016-05-27T17:53:14Z

@chouseknecht I realized that this module doesn't allow for any easy way to disconnect containers from an existing network without having to specify all of the containers in the network. For example, removing a container from the default bridge network after having added it to a user-defined network. I was thinking the simplest way to do this would be to have a disconnected parameter that mirrors the connected flag but for ensuring a container is disconnected. What do you think?

bcoca · 2016-06-08T18:20:59Z

cloud/docker/docker_network.py

+
+  state:
+    description:
+      - <


i think you want > here and in other spots

chouseknecht · 2016-06-08T18:36:55Z

@keitwb - we reviewed this today in the community meeting, New Module Review. Here are notes on required updates before this module can be accepted.

There is no method for updating an existing network. If a network exists with a configuration not matching the requested configuraiton, nothing happens. However, users will get an 'ok' and think things were all good when in fact the configurations do not match. The module should compare, and if different remember which containers are connected, disconnect them, blow the network away, recreate, and then reconnect containers.
If connected is specified, it's canonical unless incremental is True. That's fine. However, if connected is not specified (None), the connected list should be populated with the currently connected containers so that the module does not automaticallyremove all containers from the network.

gregdek · 2016-06-09T03:25:35Z

@keitwb A friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open pending your changes. When you do address the issues, please respond with ready_for_review in your comment, so that we can notify the maintainer.

[This message brought to you by your friendly Ansibull-bot.]

Implementation of https://github.com/ansible/proposals/blob/master/docker/docker_network.md

keitwb · 2016-06-11T22:29:13Z

@chouseknecht I did the change for the empty/null connected param, but I'm not quite following your first point above. The module will update existing networks and should converge the network state to what is specified in the params. If you add more containers to the connected param than currently exist in the network it will add them to the existing network. Can you give a specific example of what you mean?

gregdek · 2016-06-24T15:26:17Z

@keitwb Another friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open. If you have addressed the issues and believe it's ready for review, please comment with the text "ready_for_review". If we don't hear from you within another 14 days, we will close this pull request.

[This message brought to you by your friendly Ansibull-bot.]

keitwb · 2016-06-24T15:32:27Z

ready_for_review

gregdek · 2016-06-24T21:26:10Z

Thanks @keitwb for this new module. When this module receives 'shipit' comments from two community members and any 'needs_revision' comments have been resolved, we will mark for inclusion.

[This message brought to you by your friendly Ansibull-bot.]

gregdek · 2016-08-12T15:30:41Z

Thanks @keitwb for this PR. Unfortunately, it is not mergeable in its current state due to merge conflicts. Please rebase your PR. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review.

For help on how to do this cleanly please see http://docs.ansible.com/ansible/community.html#contributing-code-features-or-bugfixes

[This message brought to you by your friendly Ansibull-bot.]

jctanner · 2016-08-12T15:38:26Z

replaced by #4404

@keitwb thank you for your efforts on this.

keitwb · 2016-08-12T15:39:58Z

An updated version of this was merged in #4404 so I'm closing this.

gregdek added cloud new_plugin needs_revision labels May 24, 2016

keitwb changed the title ~~docker_network Module~~ New Module: docker_network May 24, 2016

keitwb mentioned this pull request May 24, 2016

Docker network management module ansible/ansible-modules-extras#1859

Closed

chouseknecht reviewed May 25, 2016
View reviewed changes

bcoca added community_review core_review and removed needs_revision labels May 26, 2016

This was referenced May 26, 2016

Agenda for Public Core Meeting - 2016-05-26 15:00 UTC ansible/community#107

Closed

Standing Agenda, New Modules Meeting ansible/community#92

Closed

gregdek removed the community_review label May 26, 2016

bcoca reviewed Jun 8, 2016
View reviewed changes

cloud/docker/docker_network.py

state:

description:

- <

Copy link

Member

bcoca Jun 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think you want > here and in other spots

bcoca added the needs_revision label Jun 8, 2016

gregdek removed the core_review label Jun 8, 2016

docker_network Module

57f3258

Implementation of https://github.com/ansible/proposals/blob/master/docker/docker_network.md

gregdek added the pending_action label Jun 24, 2016

gregdek added community_review and removed needs_revision labels Jun 24, 2016

chouseknecht mentioned this pull request Aug 12, 2016

Adding docker_network module. #4404

Merged

gregdek added needs_revision and removed community_review labels Aug 12, 2016

jctanner closed this Aug 12, 2016

jctanner removed needs_revision pending_action labels Aug 12, 2016

keitwb deleted the docker_network branch August 12, 2016 15:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Module: docker_network #3728

New Module: docker_network #3728

keitwb commented May 24, 2016 •

edited

gregdek commented May 24, 2016

chouseknecht May 25, 2016

chouseknecht commented May 25, 2016

keitwb commented May 25, 2016 •

edited

keitwb commented May 27, 2016

bcoca Jun 8, 2016

chouseknecht commented Jun 8, 2016

gregdek commented Jun 9, 2016

keitwb commented Jun 11, 2016

gregdek commented Jun 24, 2016

keitwb commented Jun 24, 2016

gregdek commented Jun 24, 2016

gregdek commented Aug 12, 2016

jctanner commented Aug 12, 2016

keitwb commented Aug 12, 2016

New Module: docker_network #3728

New Module: docker_network #3728

Conversation

keitwb commented May 24, 2016 • edited

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

SUMMARY

gregdek commented May 24, 2016

chouseknecht May 25, 2016

Choose a reason for hiding this comment

chouseknecht commented May 25, 2016

keitwb commented May 25, 2016 • edited

keitwb commented May 27, 2016

bcoca Jun 8, 2016

Choose a reason for hiding this comment

chouseknecht commented Jun 8, 2016

gregdek commented Jun 9, 2016

keitwb commented Jun 11, 2016

gregdek commented Jun 24, 2016

keitwb commented Jun 24, 2016

gregdek commented Jun 24, 2016

gregdek commented Aug 12, 2016

jctanner commented Aug 12, 2016

keitwb commented Aug 12, 2016

keitwb commented May 24, 2016 •

edited

keitwb commented May 25, 2016 •

edited